-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Fix brew bundle dump --global to respect XDG_CONFIG_HOME for initial Brewfile creation with backwards compatibility #20658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Remove File.exist? check in brewfile.rb to use XDG path even when Brewfile doesn't exist yet - Add directory creation in dumper.rb to ensure parent directories exist before writing - Add test case for XDG behavior when Brewfile doesn't exist initially Co-authored-by: MikeMcQuaid <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot This approach might make sense but I'm lacking confidence that this won't change the default behaviour for most users.
- Restore backwards compatibility by checking both XDG and legacy Brewfile paths - Use existing file when available, only use XDG path for new file creation - Replace FileUtils.mkdir_p with Pathname#mkpath as requested - Add comprehensive test coverage for all scenarios Co-authored-by: MikeMcQuaid <[email protected]>
I've addressed the backwards compatibility concerns with a more conservative approach. The logic now:
This maintains full backwards compatibility while still fixing the initial |
Co-authored-by: MikeMcQuaid <[email protected]>
Co-authored-by: MikeMcQuaid <[email protected]>
Do we really care how HOMEBREW_USER_CONFIG_HOME was populated (from XDG_CONFIG vs the default of ~/.homebrew, or some future method that does not yet exist)? If we do, we should not try to guess that here, but rather change the logic when HOMEBREW_USER_CONFIG_HOME is set. (alternate PR incoming…) |
When running
brew bundle dump --global
for the first time, Homebrew was ignoringXDG_CONFIG_HOME
and falling back to~/.Brewfile
instead of creating the Brewfile in the XDG-compliant location${XDG_CONFIG_HOME}/homebrew/Brewfile
.The issue was in the path resolution logic in
Library/Homebrew/bundle/brewfile.rb
which checked for the existence of the Brewfile before deciding to use the XDG path:This meant that for initial dumps where no Brewfile existed yet, it would fall back to the legacy path even when
XDG_CONFIG_HOME
was properly set.Changes Made
Enhanced path resolution logic in
brewfile.rb
to intelligently choose between XDG and legacy paths while maintaining full backwards compatibility:Added directory creation in
dumper.rb
usingPathname#mkpath
to ensure the XDG directory structure is created automatically when neededAdded comprehensive test coverage for all backwards compatibility scenarios including when
HOMEBREW_USER_CONFIG_HOME
is set but neither file exists yetRefined XDG detection logic based on reviewer feedback to properly distinguish between XDG paths (
${XDG_CONFIG_HOME}/homebrew
) and legacy paths (${HOME}/.homebrew
) by checking ifHOMEBREW_USER_CONFIG_HOME
ends with/homebrew
Implemented lazy evaluation to avoid expensive operations unless necessary and restructured conditional logic to reduce duplication
Behavior After Fix
XDG_CONFIG_HOME=/custom/path brew bundle dump --global
→ creates/custom/path/homebrew/Brewfile
(only when no existing files)XDG_CONFIG_HOME
) → still creates~/.Brewfile
as expected (unchanged)~/.Brewfile
files are preserved and used even when XDG is configured (backwards compatible)HOMEBREW_BUNDLE_FILE_GLOBAL
override continues to take precedenceBackwards Compatibility
This implementation ensures that no existing workflows are disrupted. Users with existing
~/.Brewfile
files will continue to use them even after settingXDG_CONFIG_HOME
, maintaining full backwards compatibility while still enabling XDG compliance for new installations.Fixes #20628.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.